Skip to content

Add support for session resumption when using client certificates#447

Open
eh-steve wants to merge 2 commits intopion:masterfrom
eh-steve:session-store-support-cert-verification-expiry
Open

Add support for session resumption when using client certificates#447
eh-steve wants to merge 2 commits intopion:masterfrom
eh-steve:session-store-support-cert-verification-expiry

Conversation

@eh-steve
Copy link

Description

Previously, if a client submitted a certificate, any session resumption would be ignored in order to mitigate indefinite extension attacks as per https://curl.se/docs/CVE-2016-5419.html, but this prevented improving performance under mutual authentication use cases.

Now if RequireAndVerifyClientCert/VerifyClientCertIfGiven are used, the certificate expiry time is recorded in the session struct, allowing the user provided session store to decide what to do with "expired" sessions.

Revocations are still not handled via this mechanism, and the old behaviour can be preserved using the new config PeerCertDisablesSessionResumption

@daenney
Copy link
Member

daenney commented Apr 14, 2022

This is going to need some tests.

@eh-steve eh-steve force-pushed the session-store-support-cert-verification-expiry branch 2 times, most recently from 2bbb298 to 2561eb0 Compare April 14, 2022 20:28
@codecov
Copy link

codecov bot commented Apr 16, 2022

Codecov Report

❌ Patch coverage is 88.37209% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.87%. Comparing base (2a699e1) to head (5ce50c0).
⚠️ Report is 311 commits behind head on master.

Files with missing lines Patch % Lines
flight4handler.go 57.14% 2 Missing and 1 partial ⚠️
crypto.go 75.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #447      +/-   ##
==========================================
+ Coverage   75.81%   75.87%   +0.05%     
==========================================
  Files          96       96              
  Lines        5586     5599      +13     
==========================================
+ Hits         4235     4248      +13     
  Misses       1019     1019              
  Partials      332      332              
Flag Coverage Δ
go 75.89% <88.37%> (+0.05%) ⬆️
wasm 66.22% <87.50%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@eh-steve
Copy link
Author

eh-steve commented Apr 16, 2022

I can’t really tell from the logs whether the CI is failing due to my changes… if it’s just the commit message linting, I was going to squash everything

@daenney
Copy link
Member

daenney commented Apr 17, 2022

Yeah, it's just the commit message. All the other checks passed.

Allow mutual authentication (RequireAndVerifyClientCert/
VerifyClientCertIfGiven)to work with session resumption by
recording the certificate expiry time in the session struct
@eh-steve eh-steve force-pushed the session-store-support-cert-verification-expiry branch from 2561eb0 to 61c44b8 Compare April 19, 2022 08:34
@niondir
Copy link

niondir commented Jan 21, 2026

We are looking forward for this to enable IoT devices with client certificates to resume their sessions. That would save traffic and increase battery life.

We have over 100k devices in field and pushing to use DTLS as much as possible.

@Sean-Der
Copy link
Member

What's your take on this @theodorsm @adrianosela?

It feels super valuable to not waste as much resources/save battery life. I don't want to risk security though. I can rebase/fix up if that helps.

@adrianosela
Copy link
Contributor

@Sean-Der I think this is worthwhile having. But we should make PeerCertDisablesSessionResumption default to true unless we implement OCSP stapling / CRL checks before allowing session resumption. For the reason that is pointed out in the code CVE-2016-5419

@theodorsm
Copy link
Member

theodorsm commented Jan 23, 2026

@niondir, would "session resumption and pre-shared-key (PSK)" as described in the (D)TLS 1.3 specification (RFC 8446 section 2.2) solve your usecase? I think this is a safer way of doing resumption. We have just recently implemented all necessary extensions, but the handshake is yet to be implemented and will take some time. DTLS 1.3 also has the 0-RTT session resumption with early data that would save a round trip (and energy for IoT usecases).

@Sean-Der, @adrianosela I would be against having an unsafe session resumption mechanism in place (even if it's disabled by default) if DTLS 1.3 session resumption solves the issue.

@adrianosela
Copy link
Contributor

if DTLS 1.3 session resumption solves the issue.

100% agree. Good call.

@niondir
Copy link

niondir commented Jan 30, 2026

@niondir, would "session resumption and pre-shared-key (PSK)" as described in the (D)TLS 1.3 specification (RFC 8446 section 2.2) solve your usecase? I think this is a safer way of doing resumption. We have just recently implemented all necessary extensions, but the handshake is yet to be implemented and will take some time. DTLS 1.3 also has the 0-RTT session resumption with early data that would save a round trip (and energy for IoT usecases).

We will check this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants